Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v626][RF] Backports of RooFit PRs to v6-26-00-patches: Part 3 #9710

Conversation

Zeff020 and others added 7 commits January 26, 2022 12:24
Fix the following compilation error on Windows:
```
testHistFactory.obj : error LNK2019: unresolved external symbol "class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const & __cdecl RooFit::tmpPath(void)" (?tmpPath@RooFit@@YAAEBV?$basic_string@DU?$char_traits@D@std@@v$allocator@D@2@@std@@xz) referenced in function "public: virtual void __cdecl HFFixture_ModelProperties_Test::TestBody(void)" (?TestBody@HFFixture_ModelProperties_Test@@UEAAXXZ)
testHistFactory.exe : fatal error LNK1120: 1 unresolved externals
```
The first implementation of RooFitH3 had a number of shortcomings, which this
PR addresses.

In detail:

  * It is now possible to read JSON files independent of the ordering
  * A priority mechanism has been implemented for importers and exporters
  * Duplicate and dead code has been removed
  * Many small bugs have been fixed
  * The JSONInterface has been made public and moved from Detail to
    Experimental, so users can write their own importers & exporters
  * The two unit tests have been fixed

Co-authored-by: Nicolas Morange <nicolas.morange@cern.ch>
Co-authored-by: Jonas Rembser <jonas.rembser@cern.ch>
The usecase for this overload is to transfer the ownership of RooAbsArgs
from `std::unique_ptr` to another RooAbsArg, without having to call
`release()` on the smart pointer, which is a code smell because
ownership should always be clear without the smart pointer having to
release it.
Calling `new` and `delete` is now completely avoided in RooFitHS3, and
`std::unique_ptr` is always used instead.
Unlike instance methods, a `property` object in Python 2 has no
`__func__` attribute and can be reassigned to a new class directly.
  * Add two tutorials `rf711_lagrangianmorph` and `rf712_lagrangianmorphfit` to
    demonstrate the usage of the `RooLagrangianMorphFunc` class

  * The commit also includes an update to `rf512_wsfactory_oper` showing an
    example for the new options of `taylorexpand` and `lagrangianmorph`

  * formatting of `tutorials/roofit/rf710_roopoly.py`

  * The attribute for new physics couplings in the `RooLagrangianMorphFunc`
    class is changed from `NP` to `NewPhysics` to avoid confusion with other
    abbreviations

  * The `lagrangianmorph` factory interface is update to accept arguments in
    any order
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@guitargeek guitargeek changed the title [v626][RF] Backports of RooFit PRs to v6-26-00-patches: Part 2 [v626][RF] Backports of RooFit PRs to v6-26-00-patches: Part 3 Jan 26, 2022
This is presumbaly a bit controversial.

safeDeleteList remove elements in order in a RooAbsCollection,
starting with the ones that only have clients and no servers.

This is a slow process, and takes 25% of CPU time on large workspace
manipulation workflows, as it takes place at each workspace::import
call. It can also lead to slow ~RooWorkspace.

The point is, I don't think this logic is needed at all.
~RooAbsArg takes care of properly breaking all the client-server links,
both uplinks and downlinks, for every object. I couldn't find a logical
case where a crash would occur if the safeDeleteList logic were to be
removed.

All RooFit tests pass after this patch. No problem for my heavy
workspace manipulation worflows either.
[RF] Include RooAbsData objects in the RooNameReg

The logic from RooAbsArg is copied into RooAbsData.

This allows to use the hash-map functionality of RooLinkedList
for RooAbsData objects, as the namePtr mechanism allows to track
renaming and therefore avoids false negatives that result in
linear scans of the collection.

In turn, this improves significantly the run-time of large workspace
imports (x2 to x4), which were dominated by embeddedData() calls.
This patch is based on the JSON tool use-case, but presumably will
significantly also improve other heavy uses of workspace import, such
as Higgs combination workspaces manipulation workflows.

The cost of one additional pointer per RooAbsData object seems a low
price to pay.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@guitargeek guitargeek merged commit dc9a695 into root-project:v6-26-00-patches Jan 26, 2022
@guitargeek guitargeek deleted the v6-26-00-patches_roofit_backports_3 branch January 26, 2022 21:08
@phsft-bot
Copy link
Collaborator

Build failed on mac1015/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2022-01-27T00:06:49.621Z] CMake Error at /Volumes/HD2/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1064 (message):

@phsft-bot
Copy link
Collaborator

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

  • [2022-01-27T08:13:54.423Z] CMake Error at C:/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1064 (message):

@phsft-bot
Copy link
Collaborator

Build failed on mac11/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2022-01-27T12:55:13.275Z] CMake Error at /Users/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1064 (message):

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2004/soversion.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2022-01-27T14:26:16.824Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1064 (message):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants